Skip to content

Allow passing HandlerFunction to run function directly #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 11, 2021
Merged

Allow passing HandlerFunction to run function directly #20

merged 1 commit into from
Oct 11, 2021

Conversation

@Ugzuzg
Copy link
Contributor

@Ugzuzg Ugzuzg commented Apr 1, 2021 •
edited
Loading

It would be useful for us to be able to pass the handler function directly to the run export when using it programmatically, as we want to have our own RIC, which loads a handler that doesn't need to know about being inside lambda.

import { run } from 'aws-lambda-ric';

import loadCustomHandler from './some-path';

const customHandler = loadCustomHandler(...);

const handler = async (event: Event, context: unknown): Promise<void> => {
  const parsedPayload = ...;
  await customHandler(parsedPayload);
}

run(handler);

@thekiwi
Copy link

thekiwi commented Jun 9, 2021

Hi @carlzogh, it seems you are the primary contributor on this repo. Are we able to get some feedback on this PR? We have some internal PRs that are blocked waiting on whether this PR will be merged or not. Thanks in advance!

@carlzogh
Copy link
Contributor

carlzogh commented Jun 10, 2021 •
edited
Loading

Thanks for your contribution @Ugzuzg!

The approach looks good to me, this would also enable the NodeJS RIC to be run similarly to the GoLang RIC by passing a static reference to a function handler.

Would you be able to add test coverage for isHandlerFunction() and the new invocation mode as part of this PR as well?

@Ugzuzg Ugzuzg force-pushed the feat/run-handler-func branch from 445072a to e6c7c58 Compare June 14, 2021 11:58
@Ugzuzg
Copy link
Contributor Author

Ugzuzg commented Jun 14, 2021

@carlzogh, I have added the tests as requested.
Also, modified the signature of the run function to match GoLang's RIC (no appRoot required when passing the handler as a function).

@Ugzuzg Ugzuzg force-pushed the feat/run-handler-func branch from e6c7c58 to 9f85619 Compare June 14, 2021 12:32
@Ugzuzg
Copy link
Contributor Author

Ugzuzg commented Jun 14, 2021

@carlzogh, forgot to run make format before pushing. Should be better now.
Can you please approve the workflow again?

@Ugzuzg Ugzuzg force-pushed the feat/run-handler-func branch from 9f85619 to f34110d Compare June 16, 2021 10:03
@Ugzuzg
Copy link
Contributor Author

Ugzuzg commented Jun 16, 2021

@carlzogh, rebased on top of the latest changes from main branch. Can you please review?

@thekiwi
Copy link

thekiwi commented Jul 21, 2021

@carlzogh any update on this? thanks in advance!

@Ugzuzg Ugzuzg force-pushed the feat/run-handler-func branch from f34110d to b60c1ab Compare September 20, 2021 11:46
@Ugzuzg Ugzuzg changed the title Allow passing HandlerFunction to run directly Allow passing HandlerFunction to run function directly Sep 20, 2021
zsombor-balogh
zsombor-balogh approved these changes Oct 11, 2021
View reviewed changes
Copy link

@zsombor-balogh zsombor-balogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

carlzogh
carlzogh approved these changes Oct 11, 2021
View reviewed changes
Copy link
Contributor

@carlzogh carlzogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zsombor-balogh zsombor-balogh merged commit 53a4f71 into aws:main Oct 11, 2021
@Ugzuzg Ugzuzg deleted the feat/run-handler-func branch October 11, 2021 15:27
@dnalborczyk dnalborczyk mentioned this pull request Oct 15, 2021
Draft
@thekiwi
Copy link

thekiwi commented Oct 27, 2021

Hey @zsombor-balogh - many thanks for reviewing and merging! Are you able to release a new version of the library which includes the change, so we can start using it?

@andclt andclt mentioned this pull request May 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@carlzogh carlzogh carlzogh approved these changes

@zsombor-balogh zsombor-balogh zsombor-balogh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Ugzuzg @thekiwi @carlzogh @zsombor-balogh